-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Switch 2 Controller Support #13327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Switch 2 Controller Support #13327
Conversation
Well, my preferred name would be just Nohzockt or Noah, no email or something special. I feel honoured that you ask me. Thanks a lot. |
Done! I also cleaned up the button reading and made a working gamepad mapping, once the left/right stick are working this is ready to review. |
Quick question. I was able to generate an SDL3.dll file with your WIP stub, would an SDL2.dll be able to be generated as well or this still a work in progress? I am wondering how these can be built for executables like emulators or steam API. |
I have some devices I can test in addition to the GC controller so I might want to add those to this PR if possible. I can create a branch off of your branch, or add them after this is merged. |
Whatever works! I can add commits to this PR if Sam doesn't get to it first. |
int size; | ||
while ((size = SDL_hid_read_timeout(device->dev, packet, sizeof(packet), 0)) > 0) { | ||
if (size > 14) { | ||
Uint64 timestamp = SDL_GetTicksNS(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block should be refactored out into a separate function to handle devices with varying capabilities and report formats.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe all NS2 devices report the same layout (passing 0 for unsupported axes like the GCN triggers) but @endrift can confirm this.
040524d
to
01f8dea
Compare
Does this support Bluetooth? What changes would need to be made for rumble and gyro support? |
I haven't looked over the code yet, but I tested the controller and discovered a few things that might want improvements:
|
Testing adding the USB ID for the Pro controller (0x2069) to this, it doesn't actually show up as functional. I haven't dug further yet. Also, Misc1 definitely shows up in testcontroller for the Xbox One controller. |
Misc2 doesn't show up in the gamepad image, but it will show up in the left-hand input list if it's mapped. |
Odd... does anything seem off with the mapping added to gamepad.c? |
Latest push is confirmed working with misc1/misc2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor things I noticed, but until calibration is figured out I don't think this will be ready.
const unsigned char DEFAULT_REPORT_DATA[] = { | ||
0x03, 0x91, 0x00, 0x0d, 0x00, 0x08, | ||
0x00, 0x00, 0x01, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF | ||
}; | ||
const unsigned char SET_LED_DATA[] = { | ||
0x09, 0x91, 0x00, 0x07, 0x00, 0x08, | ||
0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to use some enums for things like the first byte (the command) and the second byte (the direction), but that's not needed for the initial merge.
Thanks to kiddkaffeine for the Xcode updates!
Latest push addresses most of the review items, left the question for Sam and didn't touch the init commands just yet; presumably we need at least one more to get it to show us calibration data so we can probably make the change at that point. |
Are you on ndeadly's discord where a bunch of discussion about reversing has been going on? If not, it's probably a good idea to join. |
Went ahead and joined, probably won't be able to add very much under the circumstances but I'll keep up as best as I can! |
I've made a PR on your branch to get the procon working too. However, confining the bulk endpoint comms to hid.c won't be viable for this driver. Things like rumble go over that endpoint, so we need a way to plumb it through to the actual driver. |
timestamp, | ||
joystick, | ||
(Uint8) i, | ||
(packet[buttons[i].byte] & buttons[i].mask) != 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the latest change to the buttons
array, it seems fairly redundant from a purely functional standpoint.
(packet[buttons[i].byte] & buttons[i].mask) != 0 | |
(packet[3 + (i >> 3)] & (1U << (i & 7))) != 0 |
Merged the ProCon2 support and renamed this accordingly. |
I tested this PR out with the NSO GameCube controller and the init code seems to work great on Linux but doesn't seem to be happening on macOS. |
Looking over the code that's come out of ndeadly's Discord, I can see where the calibration data is stored in the SPI flash. However, without a way to plumb through the bulk endpoint to the Switch 2 driver itself, I'm not sure how we should handle this. Honesty, putting any of the NS2-specific in the hidapi code I treat as code smell. We need to plumb it through for other things, so we should figure out how to do that. @slouken, do you have any ideas? |
Also it looks like misc3 and 4 should be mapped to the click on the GC controller, per a recent commit to main. |
There currently isn't any concept of a USB endpoint or internal API to interact with it, and I'm not sure how we'd plumb that in. I'm open to ideas? Maybe just start with a hack to get it working and we can refactor from there? |
Thanks to Nohzockt for the initial libusb init and hidapi polling work!
Latest push adds misc3/misc4 bindings.
Looking over libusb's hid.c, we sort of have this via the separate input/output endpoint variables: https://github.com/libsdl-org/SDL/blob/main/src/hidapi/libusb/hid.c#L110-L111 We also have existing quirks for Xbox to claim specific interfaces that aren't typical for generic devices, so one possible path is to treat the bulk endpoint as the definitive output rather than the normal hid endpoint, and use the libusb init routine to do any reading of the bulk endpoint, assuming reading from bulk is unnecessary during regular polling, to get the factory calibration data. |
That might work, at least for now. It's pretty ugly, and I'm not sure how we'd get that calibration data to the driver. |
If we're not too worried about adding to hidapi itself maybe we can add an SDL_PropertiesID to hid_device? |
It's an internal structure, so yes, that would work. We could add SDL_hid_get_device_properties(), and pass through a second device that is the bulk channel... ? |
If I want to add audio support to the GIP driver I'll also need to pass through the isochronous endpoint to that somehow, so a way that works for both would be ideal. A "get sidechannel" function perhaps. |
It's also possible to read from the factory data section of the flash memory (0x13000-0x13FFF) containing the calibration data via the control endpoint. Would that solve some of your issues? |
Unfortunately no, the only interface that's exposed as-is is the first interrupt endpoint |
Fixes #13178.
Device init and polling works, but I haven't translated the packets yet since
the public code doesn't line up with the automatic mappingsand I'm intentionally avoiding doing any further reverse engineering myself. Happy to add patches to get this PR working though!udev rule for Linux:
CC @Nohzockt: If you have a preferred name/email for
Co-authored-by
I'll apply it to the patch that implements the init/polling work.